-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Block Library - Heading]: Add heading levels as block variations #33811
Conversation
Size Change: +64 B (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
This is nice! There are a lot of headings when just searching for /h, but the generic "Heading" shows up first, so it seems fine. Possibly/probably a separate issue, I'm seeing the black inserter appender immediately following a Heading block, I'd assume there still being the paragraph appender (no visible plus). Is that new or the same in trunk? Still catching up here after a little AFK 😅 Finally — chosing the right heading level is important for the semantic and accessible structure of the document. For example if your site title is a h1, you should use h2s inside the document, and you should only insert an h3 if you've already inserted an h2 before that, and so on. We have the document outline to help guide you towards the correct heading level. When seeing this list, I wonder if we should be smarter about how we surface incorrect heading levels:
I wondered about showing only the allowed heading levels, but since the mechanism of choosing the right heading level is a bit obtuse that seems like the wrong approach, and that the educational route is the better path. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the code level, it looks good. It's more the decision what the best experience on the user level.
name: `heading-${ level }`, | ||
/* translators: %d: heading level. */ | ||
title: sprintf( 'Heading %d', level ), | ||
icon: <HeadingLevelIcon level={ level } />, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: @gwwar, if we had better handling for icons, we could keep put those variations in block.json
. Well, JS offers loops that make encoding those variations simpler. The downside is that variations aren't exposed on the server.
title: sprintf( 'Heading %d', level ), | ||
icon: <HeadingLevelIcon level={ level } />, | ||
attributes: { level }, | ||
scope: [ 'inserter' ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to include isActive
to show different titles and icons in the block editor UI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this can be discussed as well. I intentionally left it out for start, as I thought it was not so much needed -but of course that was a personal opinion 😄
This reminds me that |
Yes, I felt that keeping the default Heading would be easier for users to select, if they don't want another specific level as it's shown at the top of the list this way. Other should chime in to this as well.
Agreed! |
That would be really useful for sure. If we make this a requirement it would need some exploration to expand the inserter API to do some additional work and display such notices. |
If we were to change the default Heading to "Heading 2", we should probably hide Heading 1 from the slash command or the sorting order would get weird. Arguably you shouldn't insert H1s anyway, and if you do need to, you can still insert a h2 and change it. Not sure this is the best approach, but we definitely don't want you inserting H1s by default. Designwise, we could also reduce the prominence of the level, potentially, by showing the number as a boxed chip or something that looks more like a property of the block, than a discrete block itself. |
It seems we should only allow |
ca47ee0
to
ad2209b
Compare
Until now all the enhancements and use cases were to make things (blocks, etc..) easier to discover through the inserter, but here comes a use case (under discussion) that points towards the opposite direction, to show some blocks based on a
This is not possible with the current API for the above reasons (promotes discoverability) and we cannot even hack our way around it (and probably we shouldn't 😄 ). I wanted to find at least another use case to see whether it makes sense to augment our API for the I explored some approaches that could work for the above use cases, but in order to only show only one occurrence of the block and its shortcuts, would require to filter the results for removing duplicate suggestions from the same block. After those exploration I don't think such an API with this overhead is worth it, so I'll pause this one for now.. |
Why not? Multiple H1s are perfectly valid. |
Absolutely, you need to be able to. I'm thinking mainly in terms of classic theme structure where the site title is an H1, meaning everything that follows should usually be a level lower in the hierarchy to indicate its proper relationship. Did I get that wrong? |
Hmmm The site-title is H1, but so is the current-post title. I suppose having multiple h1 elements inside the post itself would be somewhat unusual, but not invalid depending on the scenario. 🤔 |
@ntsekouras we can contemplate having a |
A shortcuts API was my latest exploration (I started at block level, but could easily expand to something different like
This is because we prepopulate the available items to search before a user types anything. Do you think I should try it out and check the performance? Do you think there would be other use cases for this? |
Description
Resolves: #32472
This PR just tries out adding heading levels as block variations. This way when you type something like
/h4
, an available option will be shown with a heading with level 4. Noting that the way variations work now, these heading variations are also shown in the main inserter. This feels a bit bloated to me but would like some other thoughts. Regarding this an alternative would be to expand the API by adding a scope that separates main and slash inserters, but I don't think there are other use cases to justify that API change.Also noting that there is a similar transformation in place where you can type
/h3
and pressenter/return
with the only difference that there is no UI indication that this transformation will happen. You need to know about it 😄